fix(ui): resolve root-relative README markdown links to repo blob URLs#2929
fix(ui): resolve root-relative README markdown links to repo blob URLs#2929BittuBarnwal7479 wants to merge 8 commits into
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
1 Skipped Deployment
|
📝 WalkthroughSummary by CodeRabbit
WalkthroughThe PR updates README URL resolution to mark and unwrap local redirects, adds GitHub fork support across comparison data, facets, locales, schemas, and tests, refactors chart tooltip handling, introduces HLS-backed Bluesky video playback, and refreshes workflows, dependencies, and several UI/content strings. ChangesREADME URL resolution
GitHub fork comparison facets
Media embed and chart rendering
UI copy, layout, and accessibility
Workflows, dependencies, and build tooling
Possibly related issues
Possibly related PRs
Suggested reviewers
🚥 Pre-merge checks | ✅ 4✅ Passed checks (4 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
⚔️ Resolve merge conflicts
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Codecov Report❌ Patch coverage is
📢 Thoughts on this report? Let us know! |
BittuBarnwal7479
changed the title
|
Caution Review failedAn error occurred during the review process. Please try again later. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
| const result = await renderReadmeHtml(markdown, 'test-pkg', repoInfo) | ||
|
|
||
| expect(result.html).toContain('href="/package/test-pkg"') | ||
| }) |
There was a problem hiding this comment.
Is there a reason why we want this behaviour for non-markdown links?
Why not something similar to what is done for this?
npmx.dev/test/unit/server/utils/readme.spec.ts
Lines 219 to 227 in 46e7c59
There was a problem hiding this comment.
Good catch. I updated this to resolve root-relative non-markdown links via rawBaseUrl, consistent with existing relative-link handling. Markdown files still resolve via blobBaseUrl.
Local npmx routes (/package, /org, /search, etc.) are preserved separately, and I added regression tests covering both behaviors.
There was a problem hiding this comment.
Is there a reason / place where preserving Local npmx routes would be useful instead of them resolving to rawBaseUrl too?
There was a problem hiding this comment.
yes. the main case is npmjs links that the README renderer intentionally converts to local npmx routes. For example, https://www.npmjs.com/package/test-pkg becomes /package/test-pkg.
If we treated every root-relative path as a repo file, that converted route would incorrectly become rawBaseUrl/package/test-pkg.
So the updated logic preserves known npmx routes separately, while root-relative repo files like /CONTRIBUTING.md and /assets/logo.png resolve to blobBaseUrl / rawBaseUrl.
There was a problem hiding this comment.
Hmm, is there no way to differentiate a /package that was because of https://www.npmjs.com/package/test-pkg from a /package that someone wrote in their readme?
There was a problem hiding this comment.
good point. i will update this to preserve npmjs-originated links via an internal marker rather than path matching. README-authored root-relative paths now resolve normally against the repository. Added regression tests for both cases.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
server/utils/readme.ts (1)
352-358:⚠️ Potential issue | 🟠 Major | ⚡ Quick winKeep protocol-relative URLs out of the root-relative rewrite.
//cdn.example.com/file.cssalso satisfiesurl.startsWith('/'), so with repository info it becomeshttps://raw.githubusercontent.com/.../HEAD//cdn.example.com/file.cssinstead of staying external. The later protocol-relative handling never gets a chance to run.Proposed fix
- if (url.startsWith('/')) { + if (url.startsWith('/') && !url.startsWith('//')) { if (!repoInfo?.rawBaseUrl) { return url } const baseUrl = isMarkdownFile ? repoInfo.blobBaseUrl : repoInfo.rawBaseUrl return `${baseUrl}${url}` }Please add a regression case such as
[CDN](//cdn.example.com/file.css)withrepoInfoand assert the href remains protocol-relative.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@server/utils/readme.ts` around lines 352 - 358, The root-relative URL check at the beginning of the conditional block (url.startsWith('/')) is incorrectly matching protocol-relative URLs that start with //, causing them to be rewritten as repository URLs instead of remaining external. Add an additional condition to explicitly exclude protocol-relative URLs (those starting with //) before processing the root-relative URL case in the repoInfo block. Additionally, add a regression test case that verifies a markdown link with a protocol-relative URL like [CDN](//cdn.example.com/file.css) correctly preserves the protocol-relative href when repoInfo is present.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Outside diff comments:
In `@server/utils/readme.ts`:
- Around line 352-358: The root-relative URL check at the beginning of the
conditional block (url.startsWith('/')) is incorrectly matching
protocol-relative URLs that start with //, causing them to be rewritten as
repository URLs instead of remaining external. Add an additional condition to
explicitly exclude protocol-relative URLs (those starting with //) before
processing the root-relative URL case in the repoInfo block. Additionally, add a
regression test case that verifies a markdown link with a protocol-relative URL
like [CDN](//cdn.example.com/file.css) correctly preserves the protocol-relative
href when repoInfo is present.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: e84a9795-dd1d-41b7-b64c-33fc7a943e87
📒 Files selected for processing (2)
server/utils/readme.tstest/unit/server/utils/readme.spec.ts
|
@CodeRabbit review again. |
|
✅ Action performedReview finished.
|
gameroman
added
the
needs review
| const npmJsHosts = new Set(['www.npmjs.com', 'npmjs.com', 'www.npmjs.org', 'npmjs.org']) | ||
|
|
||
| const USER_CONTENT_PREFIX = 'user-content-' | ||
| const LOCAL_NPMX_REDIRECT_PREFIX = '$npmx-local:' |
There was a problem hiding this comment.
is this because resolveUrl is being called on the same link twice?
| <option v-for="s in styles" :key="s" :value="s" class="dark:bg-gray-900"> | ||
| {{ s }} | ||
| </option> | ||
| </select> |
Lunaria Status Overview🌕 This pull request will trigger status changes. Learn moreBy default, every PR changing files present in the Lunaria configuration's You can change this by adding one of the keywords present in the Tracked Files
Warnings reference
|
BittuBarnwal7479
force-pushed
the
codex/fix-readme-root-relative-md-links
branch
from
9d5d82c to
e7c9284
Compare
There was a problem hiding this comment.
Actionable comments posted: 8
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In @.github/workflows/chromatic.yml:
- Around line 37-40: The Chromatic workflow is using an unsupported action input
for the Storybook build output, so update the chromaui/action configuration to
match Chromatic’s documented inputs. In the workflow job that uses
chromaui/action, replace the current outputDir usage with storybookBuildDir, and
make sure vp run build-storybook passes the output directory via buildCommand
using --output-dir so Chromatic can locate the generated Storybook files.
In @.github/workflows/release-tag.yml:
- Around line 103-106: The release workflow is checking out the moving release
branch instead of the immutable tag created by the tag job, so update the
checkout in the publish step to use the version output from the tag job. In the
release-tag workflow, fix the actions/checkout configuration so the publish job
uses needs.tag.outputs.version (the minted tag) rather than ref: release,
keeping the published connector aligned with the GitHub release and npm
provenance.
In `@app/components/global/BlueskyPostEmbed.client.vue`:
- Around line 120-125: Validate the external URL before assigning it to the
Bluesky embed anchor’s href: in BlueskyPostEmbed.client.vue, both postUrl (from
props.url) and post.embed.external.uri can be untrusted, so add an allow-list
check for only http: and https: before rendering the link. Update the anchor
binding and the related external link rendering around the postUrl usage and the
post.embed.external.uri usage so unsafe schemes fall back to a safe value
instead of being bound directly.
- Around line 124-125: The Bluesky embed anchor is still covering the entire
card because the pseudo-element on the link keeps the hit area stretched across
the embed. Update BlueskyPostEmbed.client.vue so the border/hover styling is
applied to the outer card container instead of the anchor, and scope the actual
link to just the icon/title area in the embed markup. Make sure the relevant
link element and container styling in the BlueskyPostEmbed component no longer
overlap the VideoPlayer or other in-card controls.
In `@i18n/locales/it-IT.json`:
- Around line 87-194: The Italian locale’s command_palette block is missing the
keyboard_shortcuts legend entries, so add the new navigate, select, and close
keys alongside the existing command_palette strings in it-IT.json. Use the
command_palette.keyboard_shortcuts structure from the other locale updates as
the reference so the CommandPalette UI can resolve these labels without falling
back to English or missing keys.
- Around line 35-36: The Italian command palette help text in the
command_palette_description entry is missing the macOS-specific shortcut hint,
so update this locale copy to match the other locales by distinguishing Mac from
Windows/Linux instead of always showing {ctrlKey}+K. Keep the wording aligned
with the existing command_palette and command_palette_description keys, and
ensure the text renders the macOS shortcut as ⌘K while preserving Ctrl+K for
other platforms.
In `@i18n/locales/ja-JP.json`:
- Around line 732-744: The locale strings for trend strength are using direction
terms instead of intensity, so update the translations for trend_strong and
trend_weak in the ja-JP JSON to reflect strength buckets rather than
upward/downward direction. Keep the wording aligned with the existing semantics
used by the trend-related labels in this file, and leave the surrounding keys
such as trend_undefined and analysis unchanged.
In `@modules/security-headers.ts`:
- Around line 53-54: The CSP in security-headers needs to allow the Bluesky
video hosts in media-src, not just video-src, because VideoPlayer.vue can fall
back to native <video> loading when Hls.isSupported() is false. Update the
media-src directive alongside the existing video-src entries to include
video.bsky.app and video.cdn.bsky.app so Safari/native HLS requests are not
blocked.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: f7e8afd3-ec12-4ae7-aefe-acd130ac5226
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (53)
.github/workflows/autofix.yml.github/workflows/chromatic.yml.github/workflows/ci.yml.github/workflows/dependency-diff.yml.github/workflows/deploy-canary.yml.github/workflows/lunaria.yml.github/workflows/mirror-tangled.yml.github/workflows/release-pr.yml.github/workflows/release-tag.yml.github/workflows/zizmor.yml.github/zizmor.ymlCONTRIBUTING.mdapp/components/Button/Base.vueapp/components/CommandPalette.client.vueapp/components/Link/Base.vueapp/components/Package/Compatibility.vueapp/components/Package/Dependencies.vueapp/components/Package/TimelineChart.vueapp/components/Package/TrendsChart.vueapp/components/Package/Versions.vueapp/components/Package/WeeklyDownloadStats.vueapp/components/VideoPlayer.vueapp/components/global/BlueskyPostEmbed.client.vueapp/composables/useChartTooltipPosition.tsapp/composables/useFacetSelection.tsapp/composables/usePackageComparison.tsapp/pages/index.vueapp/pages/package-code/[[org]]/[packageName]/v/[version]/[...filePath].vueapp/pages/package-docs/[...path].vueapp/utils/compare-scatter-chart.tsdocs/app/components/BadgeGeneratorParameters.vuedocs/content/2.guide/6.badges.mddocs/package.jsoni18n/locales/cs-CZ.jsoni18n/locales/en.jsoni18n/locales/it-IT.jsoni18n/locales/ja-JP.jsoni18n/schema.jsonmodules/security-headers.tsnuxt.config.tspackage.jsonpnpm-workspace.yamlpublic/robots.txtserver/utils/embed-downloads-svg.tsshared/types/comparison.tsshared/utils/trends-chart.tstest/nuxt/components/CommandPalette.spec.tstest/nuxt/components/compare/FacetSelector.spec.tstest/nuxt/composables/use-package-comparison.spec.tstest/unit/a11y-component-coverage.spec.tstest/unit/app/composables/use-chart-tooltip-position.spec.tstest/unit/shared/utils/trends-chart.spec.tsuno.config.ts
💤 Files with no reviewable changes (2)
- test/unit/app/composables/use-chart-tooltip-position.spec.ts
- app/composables/useChartTooltipPosition.ts
✅ Files skipped from review due to trivial changes (8)
- .github/workflows/zizmor.yml
- app/pages/index.vue
- pnpm-workspace.yaml
- CONTRIBUTING.md
- docs/app/components/BadgeGeneratorParameters.vue
- .github/zizmor.yml
- test/nuxt/components/compare/FacetSelector.spec.ts
- public/robots.txt
|
sorry for the force push. |
3 participants
🔗 Linked issue
#2928
🧭 Context
Steps to Reproduce
Recording.2026-06-17.174239.mp4
📚 Description
On package readme.ts page, root-relative markdown links such as /CONTRIBUTING.md are resolved as local npmx routes instead of repository files.